Skip to content

feat: add BedrockTransport + wire build_kwargs#13467

Closed
kshitijk4poor wants to merge 1 commit intomainfrom
feat/bedrock-transport
Closed

feat: add BedrockTransport + wire build_kwargs#13467
kshitijk4poor wants to merge 1 commit intomainfrom
feat/bedrock-transport

Conversation

@kshitijk4poor
Copy link
Copy Markdown
Collaborator

@kshitijk4poor kshitijk4poor commented Apr 21, 2026

Summary

PR 6 of the provider transport refactor (PRs 1-5: #12975, #13073, #13366, #13430, #13447).

Fourth and final transport — completes the transport layer with all 4 api_modes covered. All transport methods wired to production paths.

What ships

agent/transports/bedrock.py — BedrockTransport (155 lines)

All transport methods wired:

Method Site in run_agent.py
build_kwargs() _build_api_kwargs bedrock branch (L6713)
normalize_response() Main normalize loop — new bedrock_converse branch (L10828)
validate_response() Response validation — new bedrock_converse branch (L9389)
finish_reason Finish reason extraction — new bedrock_converse branch (L9559)
convert_messages() / convert_tools() Called internally via build_kwargs()

normalize_response() handles two shapes: raw boto3 dicts (from direct converse calls) and already-normalized SimpleNamespace with .choices (from the dispatch site at L5169).

The truncation path (L9588) intentionally groups bedrock with chat_completions — both have the same response.choices shape because normalize_converse_response already runs at the dispatch site.

Transport coverage — complete

api_mode Transport build_kwargs normalize validate finish_reason
anthropic_messages AnthropicTransport
codex_responses ResponsesApiTransport
chat_completions ChatCompletionsTransport
bedrock_converse BedrockTransport

Test plan

  • 17 new tests (build_kwargs: 3, convert_tools: 1, validate: 4, map_finish_reason: 5, normalize: 2, registration: 2)
  • 231 bedrock/converse/transport tests pass (0 failures)

@kshitijk4poor kshitijk4poor force-pushed the feat/bedrock-transport branch from 31e0516 to 937ff26 Compare April 21, 2026 10:38
Add BedrockTransport wrapping agent/bedrock_adapter.py behind the
ProviderTransport ABC. Fourth and final transport.

Wire ALL transport methods to production paths in run_agent.py:
- build_kwargs: _build_api_kwargs bedrock branch (L6713)
- normalize_response: main normalize loop, new bedrock_converse branch
  (handles both raw boto3 dicts and already-normalized SimpleNamespace)
- validate_response: response validation, new bedrock_converse branch
- finish_reason: new bedrock_converse branch in finish_reason extraction

The truncation path (L9588) intentionally groups bedrock with
chat_completions — both have the same response.choices shape because
normalize_converse_response runs at the dispatch site.

17 new tests. 231 bedrock/converse/transport tests pass (0 failures).

PR 6 of the provider transport refactor.
@kshitijk4poor kshitijk4poor force-pushed the feat/bedrock-transport branch from 937ff26 to f56c373 Compare April 21, 2026 10:45
@kshitijk4poor kshitijk4poor added the type/feature New feature or request label Apr 21, 2026
teknium1 pushed a commit that referenced this pull request Apr 22, 2026
Fourth and final transport — completes the transport layer with all four
api_modes covered.  Wraps agent/bedrock_adapter.py behind the ProviderTransport
ABC, handles both raw boto3 dicts and already-normalized SimpleNamespace.

Wires all transport methods to production paths in run_agent.py:
- build_kwargs: _build_api_kwargs bedrock branch
- validate_response: response validation, new bedrock_converse branch
- finish_reason: new bedrock_converse branch in finish_reason extraction

Based on PR #13467 by @kshitijk4poor, with one adjustment: the main normalize
loop does NOT add a bedrock_converse branch to invoke normalize_response on
the already-normalized response.  Bedrock's normalize_converse_response runs
at the dispatch site (run_agent.py:5189), so the response already has the
OpenAI-compatible .choices[0].message shape by the time the main loop sees
it.  Falling through to the chat_completions else branch is correct and
sidesteps a redundant NormalizedResponse rebuild.

Transport coverage — complete:
| api_mode           | Transport                | build_kwargs | normalize | validate |
|--------------------|--------------------------|:------------:|:---------:|:--------:|
| anthropic_messages | AnthropicTransport       | ✅            | ✅         | ✅        |
| codex_responses    | ResponsesApiTransport    | ✅            | ✅         | ✅        |
| chat_completions   | ChatCompletionsTransport | ✅            | ✅         | ✅        |
| bedrock_converse   | BedrockTransport         | ✅            | ✅         | ✅        |

17 new BedrockTransport tests pass.  117 transport tests total pass.
160 bedrock/converse tests across tests/agent/ pass.  Full tests/run_agent/
targeted suite passes (885/885 + 15 skipped; the 1 remaining failure is the
pre-existing test_concurrent_interrupt flake on origin/main).
@teknium1
Copy link
Copy Markdown
Contributor

Salvaged and merged via #13814 onto current main (commit 57411fc). Your authorship is preserved on the merge commit.

One adjustment from the original: the main normalize loop does NOT add a bedrock_converse branch. Bedrock's normalize_converse_response already runs at the dispatch site (run_agent.py:5189), so the response has the OpenAI-compatible .choices[0].message shape by the time the main loop sees it. Falling through to the else branch is correct and sidesteps a redundant NormalizedResponse rebuild.

With this merged, Cycle 1 transport coverage is complete — all four api_modes (anthropic_messages, codex_responses, chat_completions, bedrock_converse) now have transports wired to build_kwargs + validate_response + normalize_response. Thanks for the substantive work across the series (PRs 2-6).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants